Skip to content

fix(oauth): revoke replaced sessions and preserve token metadata - #1147

Open
EhabY wants to merge 1 commit into
mainfrom
fix/oauth-scope-session-lifecycle
Open

EhabY wants to merge 1 commit into
mainfrom
fix/oauth-scope-session-lifecycle

Conversation

@EhabY

@EhabY EhabY commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #1138's token-cleanup comment: clean up replaced OAuth sessions without forcing immediate re-login when required scopes change.

  • Keep existing expiry behavior: insufficient scopes prevent refresh, but do not invalidate the stored access token. Normal 401 recovery applies after expiry. Rename the eligibility check to canRefreshOAuthSession.
  • Fix replacement cleanup: save successful replacement first, then revoke the overwritten OAuth pair in the background (best effort) with the client registration captured before login. Login no longer waits on the old server. Failed/cancelled login, same-token reuse, and stored-session adoption do not trigger cleanup.
  • Preserve OAuth metadata on reuse: retain refresh credentials and scope data for the exact same token/deployment, rather than silently converting it to a manual-token session. If a refresh rotates the session while its stored token is being checked, the rotated session is kept.
  • Share OAuth plumbing: revocation lives in src/oauth/revocation.ts and is used by both logout and replacement. Client creation and metadata discovery move to withOAuthMetadata in src/oauth/metadataClient.ts, which token refresh also uses.
  • Request user:update_personal for refreshing expired workspace external-auth links. Inbox permissions remain optional.

No startup/remote admission gates, per-action permission maps, or recovery redesign. Users may encounter feature-specific permission errors before expiry; those do not automatically prompt for login.

Change size

Diff against the PR merge base; counts include moved code and rename edits.

Area Added Removed Net
Production (src/**) 175 110 +65
Tests (test/**) 144 17 +127
Changelog 7 3 +4
Total 326 130 +196

Validation

Single commit on top of #1134.

  • pnpm test: 189 files, 2,826 passed, 6 skipped.
  • pnpm typecheck, pnpm lint, and pnpm format:check: passed.
  • xvfb-run -a pnpm test:integration: passed on VS Code 1.105.0 and 1.141.0 before the rebase (activation/command smoke tests, not a live OAuth exchange). Not rerun after the latest cleanup.
  • Regression tests cover stored/provided token reuse, manual/provided/OAuth replacement, rotated old credentials, refresh rotation during a stored-token login, captured registration, save-before-revoke ordering, and failed/cancelled login. Existing tests cover outdated-scope refresh eligibility and logout revocation.
Approved narrow implementation plan
  1. Retain fix(oauth): request the scopes the extension and CLI actually need #1138's scope-filtered refresh paths: missing required scopes means no refresh, not immediate sign-out. Tokens with coder:all remain accepted.
  2. Rename the misleading OAuth-presence check to describe refresh eligibility.
  3. Preserve OAuth metadata only for reuse of the exact token and deployment URL; do not attach it to a replacement manual token.
  4. Capture client registration before login can change it. Reread the credentials being overwritten at save time, save successful replacement, then attempt cleanup using that snapshot. Do not revoke on failed/cancelled login, same-token reuse, or stored-session adoption.
  5. Share explicit-credential revocation between logout and replacement. Keep cleanup best effort and off the login path; no locking subsystem or background-refresh redesign.
  6. Keep compact regression coverage for the narrow contract.

Server source checked on Coder main 89d9492 and v2.38.0 c3c6a67: token responses report granted scopes; discovery lists recognized names, not guaranteed grants; revocation targets the presented token pair, not independent replacement credentials. Older servers omit scope and grant unrestricted access.

Generated by Coder Agents on behalf of @EhabY.

@EhabY
EhabY force-pushed the fix/oauth-scope-session-lifecycle branch from e8d23c6 to 8125427 Compare October 9, 2026 23:05
@EhabY EhabY changed the title fix(oauth): preserve and revoke sessions across scope upgrades fix(oauth): revoke replaced sessions and preserve token metadata Oct 9, 2026
@EhabY
EhabY force-pushed the fix/oauth-scope-session-lifecycle branch 3 times, most recently from 0e48fb6 to ccc5842 Compare October 10, 2026 16:26
Clean up replaced OAuth sessions without forcing a re-login when the
required scopes change.

- Insufficient scopes prevent refresh but keep the stored access token, so
  normal 401 recovery applies after expiry. Rename the check to
  `canRefreshOAuthSession`.
- Save a successful replacement first, then revoke the overwritten OAuth
  pair in the background with the client registration captured before
  login. Failed or cancelled logins, same-token reuse and stored-session
  adoption revoke nothing.
- Keep refresh credentials and scopes when the exact token and deployment
  are reused, instead of turning it into a manual-token session.
- Share revocation with logout through `withOAuthMetadata`, which token
  refresh also uses, and request `user:update_personal` to refresh expired
  external-auth links.
@EhabY
EhabY force-pushed the fix/oauth-scope-session-lifecycle branch from ccc5842 to 7ecec2f Compare October 10, 2026 16:30

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant